Skip to content

[codex] Fix nxplpc Arduino core builds#769

Merged
zackees merged 2 commits into
mainfrom
ci/soldr-cross-compile-linux-x86
Jun 23, 2026
Merged

[codex] Fix nxplpc Arduino core builds#769
zackees merged 2 commits into
mainfrom
ci/soldr-cross-compile-linux-x86

Conversation

@zackees

@zackees zackees commented Jun 23, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Summary

Fixes the nxplpc Arduino core build regression by aligning fbuild's LPC8xx build settings with the ArduinoCore-LPC8xx platform.txt recipe and correcting .ino preprocessing behavior.

Root Cause

fbuild's nxplpc config had drifted from the LPC Arduino core recipe: it used GNU++17, release LTO, -nostartfiles, -lc_nano, and extra ARM flags that PlatformIO/ArduinoCore-LPC8xx do not use. The .ino preprocessing path also generated setup()/loop() prototypes before Arduino.h, conflicting with LPC Arduino.h's extern "C" declarations, and stripped explicit user forward declarations from the sketch body.

Changes

  • Match nxplpc compile/link flags to ArduinoCore-LPC8xx's GCC recipe.
  • Stop auto-generating prototypes for Arduino-owned setup() and loop().
  • Preserve existing sketch forward declarations during .ino preprocessing.
  • Update LPC fixtures to include Arduino.h and use the correct linkage for the build-flags regression helper.
  • Add an ignored clean regression test that builds ~/dev/ArduinoCore-LPC8xx in-process and checks the generated compile command shape.

Validation

  • soldr cargo fmt --check
  • soldr cargo test -p fbuild-build source_scanner --lib
  • soldr cargo test -p fbuild-build nxplpc --lib
  • soldr cargo test -p fbuild-build --test nxplpc_build_flags -- --ignored
  • soldr cargo test -p fbuild-build --test nxplpc_core_compile_commands -- --ignored

Summary by CodeRabbit

Configuration

  • Updated NXP LPC8xx compiler flags (C++ standard, codegen options) and linker configuration for improved compatibility and simplified linking behavior.

Tests

  • Added integration test to validate NXP LPC8xx compile command output.
  • Enhanced test suite with proper Arduino framework header support across platform fixtures.
  • Updated sketch preprocessing validation to align with revised forward-declaration handling.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Updates NXP LPC8xx toolchain flags in nxplpc.json to match ArduinoCore-LPC8xx recipe expectations (C++ standard gnu++11, -lc instead of -lc_nano, removed LTO/float-ABI/nostartfiles flags), adds corresponding unit and integration tests, adds #include <Arduino.h> to all LPC test fixtures, and overhauls the .ino preprocessor to preserve existing forward declarations and exclude setup/loop from auto-generated prototypes.

Changes

NXP LPC toolchain flag alignment

Layer / File(s) Summary
NXP LPC build config flags
crates/fbuild-build/src/nxplpc/configs/nxplpc.json		 C++ standard changed from gnu++17 to gnu++11, linker_flags trimmed to --gc-sections only, -lc_nano replaced with -lc, and profiles.release drops LTO flags leaving only -Os.
Unit tests for updated compiler/linker flags
crates/fbuild-build/src/nxplpc/mcu_config.rs		 compiler_flags_target_cortex_m0plus asserts -std=gnu++11 present and -mfloat-abi=soft absent; new linker_flags_match_arduino_core_recipe test asserts -Wl,--gc-sections and -lc present, -nostartfiles and -lc_nano absent.
Integration test: compile_commands.json validation
crates/fbuild-build/tests/nxplpc_core_compile_commands.rs		 Ignored test builds lpc845brk with generate_compiledb: true, then inspects generated compile_commands.json to assert required and forbidden flags matching ArduinoCore recipe.
Add #include <Arduino.h> to LPC test fixtures
tests/platform/lpc804/lpc804.ino, tests/platform/lpc845/lpc845.ino, tests/platform/lpc845brk/lpc845brk.ino, tests/platform/lpcxpresso804/lpcxpresso804.ino, tests/platform/lpcxpresso845max/lpcxpresso845max.ino, tests/platform/lpc845_build_flags/src/main.ino		 All LPC platform test fixtures gain #include <Arduino.h>; main.ino additionally changes check_flag_no_op to extern "C" linkage.

.ino preprocessor: prototype and forward-declaration overhaul

Layer / File(s) Summary
Source scanner: remove forward-decl stripping, skip entry-point prototypes
crates/fbuild-build/src/source_scanner.rs		 Removes the pipeline step that parsed and stripped existing forward declarations from concatenated sketch source; deletes find_existing_forward_declarations, collect_forward_declarations, and has_descendant_kind helpers; adds is_arduino_entry_point_signature to exclude setup/loop from auto-generated prototypes.
Source scanner tests for revised preprocessing
crates/fbuild-build/src/source_scanner/tests.rs		 Multi-.ino ordering tests switch from find to rfind for position checks; new assertions confirm setup/loop are absent from auto-generated prototypes; new test verifies user-provided forward declarations are preserved in output.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • FastLED/fbuild#570: Directly related — that PR introduced ArduinoCore-LPC8xx vendoring which drives the -lc vs -lc_nano and -nostartfiles linker expectations being tested here.
  • FastLED/fbuild#657: Directly related — both PRs modify the .ino prototype/forward-declaration logic in source_scanner.rs, with #657 introducing tree-sitter parsing that this PR builds upon.
  • FastLED/fbuild#763: Directly related — both PRs add #include <Arduino.h> to overlapping LPC platform .ino test fixtures.

Poem

🐇 Hop hop, the flags have changed today,
gnu++11 leads the way!
No more nano-libc small,
-lc now handles all.
setup and loop need no prototype cheer—
The rabbit refactored, the build path is clear! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 73.08% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title '[codex] Fix nxplpc Arduino core builds' directly addresses the main objective of fixing a build regression in nxplpc Arduino core builds, which is the primary focus of this changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/soldr-cross-compile-linux-x86
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch ci/soldr-cross-compile-linux-x86

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zackees
Merge remote-tracking branch 'origin/main' into codex/pr-769-resolve
1554067 
# Conflicts:
#	tests/platform/lpc845_build_flags/src/main.ino
@zackees zackees merged commit 225e587 into main Jun 23, 2026
88 of 90 checks passed
@zackees zackees deleted the ci/soldr-cross-compile-linux-x86 branch June 23, 2026 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@zackees